fix/remove-custom-progressbar by wyzula-jan · Pull Request #1178 · bec-project/bec_widgets

wyzula-jan · 2026-05-28T09:53:35Z

Description

Since this widget is present in MainWindow, it should be rock solid. This PR replaces the custom paintEvent-based progress bar with a native QProgressBar.

It also unifies progress handling for ScanProgressBar and RingProgressBar through a shared progress backend. The scan progress widget now consumes the new progress messages emitted by BEC directly, instead of decoding scan queue state and report instructions on the widget side.

One non-trivial part remains in the native progress bar styling: the dynamic stylesheet logic. This is needed to compensate for the way QProgressBar::chunk handles border radius, especially at low progress values, so the rounded progress fill keeps a visually correct curvature without falling back to custom painting.

github-actions · 2026-05-29T07:44:48Z

Benchmark comparison

Threshold: 20% (lower is better).
Result: 0 regression(s), 0 improvement(s) beyond threshold.

No benchmark regression exceeded the configured threshold.

No benchmark improvement exceeded the configured threshold.

All benchmark results

Benchmark	Baseline	Current	Change	Status
BEC IPython client with companion app	5.82713 s	5.92636 s	+1.70%	ok
BEC IPython client without companion app	1.91878 s	1.90545 s	-0.69%	ok
Import bec_widgets	0.0130001 s	0.0130994 s	+0.76%	ok
tests/unit_tests/benchmarks/test_dock_area_benchmark.py::test_add_waveform_to_dock_area	0.147591 s	0.151113 s	+2.39%	ok

…h native QProgressBar

codecov · 2026-06-02T15:22:46Z

Codecov Report

❌ Patch coverage is 91.26506% with 29 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
bec_widgets/widgets/progress/progress_backend.py	91.83%	8 Missing and 4 partials ⚠️
...idgets/progress/bec_progressbar/bec_progressbar.py	91.52%	7 Missing and 3 partials ⚠️
...widgets/widgets/progress/ring_progress_bar/ring.py	83.33%	2 Missing and 3 partials ⚠️
...gets/progress/scan_progressbar/scan_progressbar.py	93.54%	0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

Refactors the scan/device progress UI to remove custom-painted progress rendering and centralize progress-message tracking in a shared backend, improving robustness for widgets embedded in the BECMainWindow.

Changes:

Replaces the custom-painted BECProgressBar implementation with a native QProgressBar-backed widget, while preserving state coloring and label formatting.
Introduces BECProgressTracker (progress_backend.py) to unify scan/device progress subscription, task lifecycle, and snapshot emission across widgets.
Updates ScanProgressBar, Ring progress bar, and related unit tests to use the new tracker and the native progress bar behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`bec_widgets/widgets/progress/progress_backend.py`	New shared progress tracking backend (source selection, task timing, snapshots, subscriptions).
`bec_widgets/widgets/progress/scan_progressbar/scan_progressbar.py`	Migrates ScanProgressBar to use `BECProgressTracker` and forwards UI updates from snapshots.
`bec_widgets/widgets/progress/bec_progressbar/bec_progressbar.py`	Replaces custom painting/animation with a native `QProgressBar` implementation + dynamic stylesheet handling.
`bec_widgets/widgets/progress/ring_progress_bar/ring.py`	Switches scan/device progress wiring to `BECProgressTracker` and updates cleanup/disconnect behavior.
`bec_widgets/widgets/containers/main_window/main_window.py`	Adjusts status-bar scan progress sizing and configures dynamic stylesheet behavior for compact/full variants.
`tests/unit_tests/test_bec_progressbar.py`	Updates tests to assert native `QProgressBar` behavior, formatting, and dynamic stylesheet logic.
`tests/unit_tests/test_scan_progress_bar.py`	Updates ScanProgressBar tests to reflect tracker-based lifecycle and new status mappings.
`tests/unit_tests/test_ring_progress_bar_ring.py`	Updates ring progress tests to expect tracker-based dispatcher connections and cleanup behavior.
`tests/unit_tests/test_device_initialization_progress_bar.py`	Adjusts assertions to read native `QProgressBar` text formatting.
`tests/unit_tests/test_main_widnow.py`	Adds coverage for compact scan progress sizing in the main window status bar.
`bec_widgets/cli/client.py`	Updates CLI proxy docstring to match the new native progress bar implementation.

Comments suppressed due to low confidence (1)

bec_widgets/widgets/progress/scan_progressbar/scan_progressbar.py:27

Docstring uses inconsistent capitalization for widget names. Use the actual class names/casing (BECProgressBar, QProgressBar) to avoid confusion and make it searchable.

    """
    Widget to display a progress bar that is hooked up to the scan progress of a scan.
    If you want to manually set the progress, it is recommended to use the BECProgressbar or QProgressbar directly.
    """

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…empting to add QLayout "" to ScanGroupBox "", which already has a layout`

wakonig · 2026-06-06T17:29:38Z

            "open": cls.NORMAL,
            "paused": cls.PAUSED,
            "aborted": cls.INTERRUPTED,
+            "halt": cls.PAUSED,


I don't think there is a "halt" state, only "halted". Where did you get that from?

wakonig · 2026-06-06T17:30:58Z

+            "halt": cls.PAUSED,
            "halted": cls.PAUSED,
            "closed": cls.COMPLETED,
+            "user_completed": cls.PAUSED,


Now that the mapping doesn't contain the color anymore, do we really need it? It just maps an enum to a string. Why not just use the enum?

wakonig · 2026-06-06T17:32:11Z

+        config=None,
+        gui_id=None,
+        enable_dynamic_stylesheet: bool = True,
+        **kwargs,


I suggest to add a doc string, especially for the arguments that are not commonly used in BW like enable_dynamic_stylesheet

wakonig · 2026-06-06T17:44:06Z

-        elif "device_progress" in instruction:
-            device = instruction["device_progress"][0]
-            self.set_progress_source(ProgressSource.DEVICE_PROGRESS, device=device)
+        self.ui.elapsed_time_label.setText(task.time_elapsed)


when I run it in designer, I get:
in update_labels
self.ui.elapsed_time_label.setText(task.time_elapsed)
RuntimeError: Internal C++ object (PySide6.QtWidgets.QLabel) already deleted.

All widgets which use ui files are having this error. It is a problem with ui loader. We have this bug for long time.

wakonig · 2026-06-06T17:46:24Z

-        Returns:
-            dict[str, list[str]]: Dictionary with the signals for the device
+        Ring device mode follows the previous progress-widget selection order:
+        progress signals first, then hinted readback signals, then normal readback signals.


probably leftover from a codex rewrite... I don't think we should have doc string references to code that we don't have anymore. Also, the input arg docs are gone?

wakonig · 2026-06-06T17:47:31Z

-
-        Depending on what type of signal we get (progress or hinted/normal), we subscribe to different endpoints.
+        Device mode keeps the previous progress-widget behavior:
+        - if no signal is provided, use the first progress signal;


same as above... that doc string is not useful if it references code that the PR removes

wakonig · 2026-06-06T17:49:17Z

+    value: float
+    max_value: float
+    done: bool
+    status: Literal["open", "paused", "aborted", "halt", "halted", "closed", "user_completed"]


no halt status

wakonig · 2026-06-06T18:01:32Z

-            "status", "open"
+        self.progress_tracker = BECProgressTracker(self.bec_dispatcher, parent=self)
+        self.progress_tracker.progress_started.connect(self._on_progress_started)
+        self.progress_tracker.progress_updated.connect(self._on_progress_snapshot)


the scan progressbar does not reset on a new scan, only on new scan progress data. If the setup takes a bit longer (e.g for moving motors to the start position), the user may get the impression that the scan doesn't run. We can solve this either by subscribing to the scan status here and simply reset it, or we broadcast 0 / 0 on a new scan from BEC. The latter could be somewhat confusing: BEC does not know at this point the max number. Probably better to simply reset the scan progressbar in BW

wyzula-jan self-assigned this May 28, 2026

wyzula-jan force-pushed the fix/remove-custom-progressbar branch from cee4514 to dd7e029 Compare May 29, 2026 07:36

wyzula-jan force-pushed the fix/remove-custom-progressbar branch 4 times, most recently from 980e0dc to 3b4ec66 Compare June 2, 2026 13:19

fix(bec_progress_bar): replace the custom paint event progressbar wit…

cd150c0

…h native QProgressBar

wyzula-jan force-pushed the fix/remove-custom-progressbar branch 2 times, most recently from b0faeb5 to b6b768e Compare June 2, 2026 15:18

wyzula-jan mentioned this pull request Jun 2, 2026

Feature/scan bundler device progress bec-project/bec#893

Merged

wyzula-jan force-pushed the fix/remove-custom-progressbar branch 5 times, most recently from cea2f10 to 35ce5a4 Compare June 5, 2026 08:53

wyzula-jan marked this pull request as ready for review June 5, 2026 09:06

Copilot AI review requested due to automatic review settings June 5, 2026 09:06

Copilot started reviewing on behalf of wyzula-jan June 5, 2026 09:06 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

Comment thread bec_widgets/widgets/progress/bec_progressbar/bec_progressbar.py

Comment thread bec_widgets/widgets/containers/main_window/main_window.py

wyzula-jan force-pushed the fix/remove-custom-progressbar branch from 35ce5a4 to 181f530 Compare June 5, 2026 09:30

wyzula-jan requested a review from a team June 5, 2026 09:31

wyzula-jan force-pushed the fix/remove-custom-progressbar branch 5 times, most recently from b91c676 to 439da68 Compare June 5, 2026 15:08

wyzula-jan added 2 commits June 5, 2026 17:22

feat(progress): progress is tracked from bec; unified progress backend

e8c85a6

fix(scan_control): remove parent from layout to prevent `QLayout: Att…

53cf334

…empting to add QLayout "" to ScanGroupBox "", which already has a layout`

wyzula-jan force-pushed the fix/remove-custom-progressbar branch from 439da68 to 53cf334 Compare June 5, 2026 15:22

wakonig reviewed Jun 5, 2026

View reviewed changes

Comment thread bec_widgets/widgets/progress/ring_progress_bar/ring.py Outdated

wyzula-jan requested a review from wakonig June 6, 2026 09:44

wyzula-jan force-pushed the fix/remove-custom-progressbar branch from 45cb5f1 to e049d04 Compare June 6, 2026 16:53

wyzula-jan added 2 commits June 6, 2026 19:10

fix(ring): ProgressSignal fetch logic back

e4147d8

refactor(bec_progress): simplification of chunk radius calculation

30b184c

wyzula-jan force-pushed the fix/remove-custom-progressbar branch from e049d04 to 30b184c Compare June 6, 2026 17:11

wakonig requested changes Jun 6, 2026

View reviewed changes

Conversation

wyzula-jan commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark comparison

Uh oh!

codecov Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wyzula-jan commented May 28, 2026 •

edited

Loading

github-actions Bot commented May 29, 2026 •

edited

Loading

codecov Bot commented Jun 2, 2026 •

edited

Loading